Skip to content

Print expressions in a canonical form so the source kind attribute does not change the expression key - #6197

Closed
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-aoscecn
Closed

Print expressions in a canonical form so the source kind attribute does not change the expression key#6197
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-aoscecn

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

$searchParams['key'] and $searchParams["key"] are the same expression, but PHPStan tracked them as two unrelated ones: narrowing applied through one spelling was invisible through the other. The reporter hit this after running pint, which rewrote double-quoted array keys to single-quoted ones and thereby changed the analysis result (their baselined count($x) > 0 always-true errors disappeared).

The fix makes PHPStan\Node\Printer\Printer print one canonical form per value, so the expression key no longer depends on how the expression was spelled in the source.

Changes

All in src/Node/Printer/Printer.php:

  • pScalar_String() - print from the value alone instead of from the kind attribute, so 'k', "k", <<<'K'/<<<K heredocs holding k all print as 'k'. Values containing control characters keep the double-quoted escaped form so the printed key stays single-line (and stays cacheable by p()).
  • pScalar_InterpolatedString() - always print the double-quoted form, so <<<K\nx$s\nK matches "x$s".
  • pScalar_Int() - normalize KIND_HEX/KIND_OCT/KIND_BIN to decimal, so $a[0x1], $a[0b1], $a[01] match $a[1].
  • pExpr_Array() and pExpr_List() - always print the short syntax, so array(...) matches [...] and list(...) matches [...].
  • pExpr_Cast_Double() - always print (float) , so (double) and (real) match (float). ((integer)/(boolean) were already normalized by php-parser.)
  • pExpr_ConstFetch() - print true/false/null lowercase and unqualified.

Test expectations updated for the new canonical output:

  • tests/PHPStan/Rules/Arrays/DuplicateKeysInLiteralArraysRuleTest.php - (null, NULL) -> (null, null)
  • tests/PHPStan/Rules/Keywords/DeclareStrictTypesRuleTest.php - \true given -> true given

Probed and found already correct, so left unchanged: Name qualification (Foo::C vs \Foo::C, handled by the name resolver), float literals (1.0/1.00/1e0, normalized by value), numeric separators (1_000), magic constants (__LINE__), (integer)/(boolean) casts, ${x} vs {$x} interpolation syntax, and $obj->{'n'} vs $obj->n (already normalized by the existing pObjectProperty() override).

Probed, found broken, and deliberately not changed: casing of class, method, static-method and function names ($f->M() vs $f->m(), \FOO::C vs \Foo::C, STRLEN(...) vs strlen(...)). PHP resolves these case-insensitively, but PHPStan already reports every one of them as a class.nameCase / method.nameCase / staticMethod.nameCase / function.nameCase error, and lowercasing identifiers in the printer would make every error message that prints an expression display a lowercased class or method name.

Root cause

php-parser records the source spelling of a node in its kind attribute (and in String_'s heredoc/nowdoc label), and PrettyPrinter\Standard faithfully reproduces it. PHPStan\Node\Printer\Printer extends Standard and is what ExprPrinter::printExpr() - and therefore MutatingScope::getNodeKey() - uses to build expression keys. So two spellings of the same value produced two different keys, and everything keyed off them (narrowed types, isset/instanceof/is_*() specifications, invalidation) applied to only one of them.

The pattern is "the printer must canonicalize meaning, not reproduce syntax", and it affected every node type whose printing consults kind: Scalar\String_, Scalar\InterpolatedString, Scalar\Int_, Expr\Array_, Expr\List_ and Expr\Cast\Double - plus Expr\ConstFetch, where the same problem comes from PHP's case-insensitive true/false/null rather than from a kind attribute. The class already had one instance of this fix (pObjectProperty(), normalizing $obj->{'n'} to $obj->n); this extends it to the rest of the family.

The turbo extension does not reimplement the printer - pt_node_printed_expr() in turbo-ext/src/support.cpp calls back into ExprPrinter::printExpr() - so no .cpp mirror change is needed.

Test

tests/PHPStan/Analyser/nsrt/bug-15060.php contains the reporter's playground sample (the dumpType calls turned into assertType), asserting that $searchParams['test'] and $searchParams["test"] get the same narrowed type after isset(), a truthiness check, and is_array().

The same file adds one function per analogous case found in step 4, each of which failed before the fix:

  • heredocAndNowdoc() - 'test' vs "test" vs heredoc vs nowdoc
  • escapeSequences() - "a\nb" vs the equivalent heredoc
  • interpolatedString() - "x$s" vs the equivalent heredoc
  • integerBases() - 1 vs 0x1 vs 0b1 vs 01
  • arraySyntax() - [$s][0] vs array($s)[0]
  • doubleCast() - (float) vs (double)
  • constantCase() - true/TRUE/\true, null/NULL, false/FALSE

Verified by stashing the Printer.php change: the test fails with mixed instead of array<mixed, mixed> on 12 assertions, and passes with the fix. make tests (21230 tests) and make phpstan are green.

Fixes phpstan/phpstan#15060

…does not change the expression key

- `PHPStan\Node\Printer\Printer` now ignores the php-parser `kind` attribute, which only records how a node was spelled in the source. Expression keys (and therefore the scope's expression tracking) are now derived from what a node means, not from how it was typed.
- `pScalar_String()`: single-quoted, double-quoted, heredoc and nowdoc strings with the same value print identically (double-quoted escaping is kept only for values containing control characters, so the printed form stays single-line and readable).
- `pScalar_InterpolatedString()`: a heredoc with interpolation prints like the equivalent double-quoted string.
- `pScalar_Int()`: hexadecimal, octal and binary literals print as decimal.
- `pExpr_Array()` / `pExpr_List()`: `array(...)` and `list(...)` print as `[...]`.
- `pExpr_Cast_Double()`: `(double)` and `(real)` print as `(float)`.
- `pExpr_ConstFetch()`: `TRUE`/`FALSE`/`NULL` and `\true`/`\false`/`\null` print lowercase and unqualified - these are the only case-insensitive constant names in PHP.
- Updated two rule test expectations that asserted the old, source-spelling-dependent output (`NULL` -> `null`, `\true` -> `true`).
- Probed but deliberately left alone: class, method, function and property-hook name casing. PHP treats those case-insensitively too, but PHPStan already reports mismatched case as `class.nameCase` / `method.nameCase` / `staticMethod.nameCase` / `function.nameCase`, and lowercasing identifiers in the printer would degrade every error message that prints an expression.
@staabm staabm closed this Aug 8, 2026
@staabm
staabm deleted the create-pull-request/patch-aoscecn branch August 8, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Array keys using single quotes are narrowed differently than the same array key using double quotes

2 participants